Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Synchronously update internal sockets length so http.Agent pooling is used #300

Merged
merged 8 commits into from
Mar 30, 2024
Merged

Synchronously update internal sockets length so http.Agent pooling is used #300

merged 8 commits into from
Mar 30, 2024

Conversation

lukekarrys
Copy link
Collaborator

Fixes #299

This is a bit of a hack, but I went with this approach because it allows for all of the native socket pooling behavior in http.Agent to be used and connect() to keep it's async signature.

The tradeoff is manipulating the internal sockets dictionary which is readonly (according to @types/node) and therefore needs a lot of @ts-expect-error comments. An alternative approach would require overwriting the addRequest() method with custom pooling code.

The approach in this PR works by synchronously adding a fake socket to the pool so this.sockets[originName].length is accurate before any other requests are added. Then the socket is removed directly after connect() finishes.

Copy link

changeset-bot bot commented Mar 29, 2024

🦋 Changeset detected

Latest commit: 4650f93

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
agent-base Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Mar 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
proxy-agents ✅ Ready (Inspect) Visit Preview Mar 29, 2024 11:48pm

@lukekarrys
Copy link
Collaborator Author

I updated the reproduction in #299 to include more examples and when I install agent-base from this PR, it works as expected.

❯ npm i https://proxy-agents-git-fork-lukekarrys-lk-socket-pooling-tootallnate.vercel.app/agent-base.tgz
❯ node index.js
core
10 max sockets
727 total versions
1873533 total bytes
----------------------------------------
agent-base-sync
10 max sockets
727 total versions
1873533 total bytes
----------------------------------------
agent-base-async
10 max sockets
727 total versions
1873533 total bytes
----------------------------------------
https-proxy-agent
10 max sockets
727 total versions
1873533 total bytes
----------------------------------------

@TooTallNate TooTallNate merged commit e62863c into TooTallNate:main Mar 30, 2024
15 checks passed
@lukekarrys lukekarrys deleted the lk/socket-pooling branch March 30, 2024 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[agent-base] maxSockets not being respected
2 participants